Skip to content

Conversation

@jbaiter
Copy link
Contributor

@jbaiter jbaiter commented Aug 31, 2023

Adds TypeScript type definitions for all publicly exposed APIs of the official 3.43.0 build of SQLite's WASM build. Based on initial work by @schickling, @mizchi and @tomayac in #36. Fixes #1.

In addition to the type definitions, every function, struct and class has a docstring. For most APIs this mirrors the online documentation, for the sqlite.capi namespace the docstrings are abbreviated versions of the docstrings in sqlite.h, with a hyperlink to the full documentation. Docstrings for sqlite.capi functions also include the original C signature, which can be essential when dealing with pointer types.

@ipb26
Copy link

ipb26 commented Sep 30, 2023

@jbaiter This is great, thanks! Been doing some testing - looks like there's one discrepancy.

sqlite3_create_function(
db: number | Database,
functionName: string | number,
nArg: number,
eTextRep: 1,
pApp: number,
xFunc: number | ((ctx: number, nArg: number, args: number) => SqlValue),
xStep: number | ((ctx: number, nArg: number, args: number) => void),
xFinal: number | ((ctx: number) => SqlValue)) => number
)

But the sqlite wasm docs mention some changes which also came up in my testing.

The semantics of generated JS wrappers are:

xFunc(): passed (pCtx, ...values). Its return value becomes the new SQL function's result.
xStep() and xInverse() (window functions): passed (pCtx, ...values). The return value is ignored.

So I guess this would be:

sqlite3_create_function(
db: number | Database,
functionName: string | number,
nArg: number,
eTextRep: 1,
pApp: number,
xFunc: number | ((ctx: number, ...values: SqlValue[]) => SqlValue),
xStep: number | ((ctx: number, ...values: SqlValue[]) => void),
xFinal: number | ((ctx: number) => SqlValue)) => number
)

This would line up with what I'm seeing at runtime as well.

@jbaiter
Copy link
Contributor Author

jbaiter commented Oct 1, 2023

Thank you for testing the types! I've pushed a fix.

@tomayac
Copy link
Collaborator

tomayac commented Oct 1, 2023

Let me know when it's ready to be merged.

@ipb26
Copy link

ipb26 commented Oct 1, 2023

Thanks @jbaiter. Couple more things I found.

  1. SqlValue should include bigint, if I understand correctly? They work fine at runtime.
  2. Can readonly SqlValue[] be added as a possible BindingSpec type? This doesn't need to be a mutable array does it?
    /** Types of values that can be passed to/retrieved from SQLite. */
    type SqlValue =
        | string
        | number
        | null
        | Uint8Array
        | Int8Array
        | ArrayBuffer
        | bigint

    /** Specifies parameter bindings. */
    type BindingSpec =
        | SqlValue[]
        | readonly SqlValue[]
        | { [paramName: string]: SqlValue }
        /** Assumed to have binding index `1` */
        | SqlValue;

- Use `readonly T[]` argument types where possible
- Fix typo in first `xWrapped` overload
- Include `BigInt` in `SqlValue` union type
@jbaiter
Copy link
Contributor Author

jbaiter commented Oct 1, 2023

@sampbarrow

1. SqlValue should include bigint, if I understand correctly? They work fine at runtime.

Indeed it should! Fixed, thank you!

2. Can `readonly SqlValue[]` be added as a possible BindingSpec type? This doesn't need to be a mutable array does it?

Oh absolutely, thank you for pointing that out, added the modifier in some other function signatures as well 👍🏾

@tomayac

Let me know when it's ready to be merged.

That probably depends on your definition of 'ready' as a maintainer 😅 . The API surface is huge, and I've done the types to the best of my knowledge, but there's bound to be gaps and mistakes in there that I overlooked. Maybe @sgbeal could take a glance at the types and see if anything obviously wrong jumps out at him?

@chuanqisun
Copy link

Is it possible to release the typescript support with the npm "pre-release" semantic version bump, so the community can try it out in their realistic projects and provide feedback?

@tomayac
Copy link
Collaborator

tomayac commented Oct 10, 2023

Is it possible to release the typescript support with the npm "pre-release" semantic version bump, so the community can try it out in their realistic projects and provide feedback?

I'm open to doing that. How would it work in practice? On npm, our versions use 3.43.1-build1, where the first part comes straight from SQLite, and I just increase the build number suffix.

Running npm version swallows the build part:

$ npm version prerelease --preid=alpha
v3.43.1-alpha.0

Can I just add it manually again so it would read v3.43.1-build1-alpha.0 and it would work? I'm a bit afraid to just try. Does anyone have experience with this? People who want to try it should do so on purpose, I don't want to accidentally make it the default version.

@chuanqisun
Copy link

The build1 part seems unconventional for npm versioning. I'm not sure how semver prerelease would be handled if we append more text after it. My hunch is that the build1 is already being treated as a prerelease tag so the users will not get automatic patch version bumps when they use ^3.43.1, but someone please confirm this.

Since the alpha does not have any special meaning to npm, it could potentially just be the build instead. So something like npm version prerelease --preid=build would give us v3.43.1-build.0 -> v3.43.1-build.1 -> v3.43.1-build.2.

Copy link
Collaborator

@tomayac tomayac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and pre-releasing to test this in practice, as agreed on in the PR thread.

@tomayac tomayac merged commit 7154305 into sqlite:main Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeScript types

7 participants